PMM-15360: Gate OpenManager's nav and page on the switch - #5853
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## PMM-15360-om-switch-flag #5853 +/- ##
============================================================
+ Coverage 46.75% 46.77% +0.02%
============================================================
Files 431 431
Lines 45132 45162 +30
============================================================
+ Hits 21100 21124 +24
- Misses 21977 21986 +9
+ Partials 2055 2052 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
aa49daf to
0eac8af
Compare
a28f277 to
69628ee
Compare
0eac8af to
53f52f5
Compare
53f52f5 to
e7855bf
Compare
839e3dc to
1de5337
Compare
e7855bf to
1126d36
Compare
1de5337 to
be40598
Compare
5cdb1b9 to
d33bdfd
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
WalkthroughOpenManager navigation entries now appear only when Merge Risk: 🟡 Moderate · up to The route currently mounts OpenManager when the controlling setting is still loading or unavailable, so direct or shared links can expose the feature UI despite the switch being off or unresolved. This is a bounded UI gating issue; the existing role check remains, but the route should fail closed before merge. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the settings gate, fail-closed page behavior, technical-preview banner, related ticket, stacking context, and feature-build status. It is sufficiently complete for this change. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/apps/pmm/src/om/OmPage.tsx (1)
112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an MUI primitive or remove the wrapper.
This
divonly composeschildren. Replace it withBox, or remove the wrapper when no DOM wrapper is required.As per coding guidelines, use MUI and
@percona/peak-uicomponents for layout and composition inui/apps/pmm/**/*.{ts,tsx}.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1267792-e20d-4a9e-8b91-7db43656bb37
📒 Files selected for processing (2)
ui/apps/pmm/src/contexts/navigation/navigation.provider.tsxui/apps/pmm/src/om/OmPage.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.
be40598 to
04f695f
Compare
Adds the settings flag as a second gate alongside the MongoDB-monitored check for OpenManager's nav entry, reusing the existing pattern for hiding MongoDB-only nav entries rather than inventing a second mechanism. OmPage now fails closed on the settings flag too, not just isPMMAdmin: a saved or shared link has to answer "switched off" rather than the API's raw FailedPrecondition or the unauthorized card, which would misreport a disabled feature as a permissions problem to an admin who has every right to be here. Also adds the technical-preview banner (dismissible per browser via localStorage, not a setting), with an explicit color override on its close button -- @percona/peak-ui's MuiAlert theme colors the icon/message slots via theme.palette[severity].contrastText but not .MuiAlert-action, so the default close button was rendering almost invisibly against the warning background. Stacks on PMM-15326-om-ui-nav (needs OmPage/navigation.provider.tsx) and this ticket's own settings-flag PR, merged in here since neither is available on a single common upstream branch yet. Signed-off-by: Pawel Lebioda <pawel.lebioda@percona.com>
d33bdfd to
eb0ae4e
Compare
navigation.provider.tsx: settings.omEnabled -> settings?.omEnabled, matching the optional-chaining the sibling backupManagementEnabled check already uses -- settings can be null before the first fetch resolves. settings.provider.tsx: the anonymous-user fallback CombinedSettings literal was missing omEnabled, now required by the type PMM-15360- om-switch-flag added. Defaults to false, matching every other flag in that fallback -- an anonymous session is never a PMM admin, so it would never see OM's nav entry regardless. Signed-off-by: Pawel Lebioda <pawel.lebioda@percona.com>
2 participants
What
Adds the settings flag as a second gate alongside the MongoDB-monitored check for OpenManager's nav entry, reusing the existing pattern for hiding MongoDB-only nav entries rather than inventing a second mechanism.
OmPagenow fails closed on the settings flag too, not justisPMMAdmin: a saved or shared link has to answer "switched off" rather than the API's rawFailedPreconditionor the unauthorized card, which would misreport a disabled feature as a permissions problem to an admin who has every right to be here.Also adds the technical-preview banner (dismissible per browser via
localStorage, not a setting), with an explicit color override on its close button —@percona/peak-ui'sMuiAlerttheme colors the icon/message slots viatheme.palette[severity].contrastTextbut not.MuiAlert-action, so the default close button was rendering almost invisibly against the warning background.Ticket: PMM-15360
Stacking
Third of three PRs for PMM-15360. Stacks on
PMM-15326-om-ui-nav(#5818, needsOmPage.tsx/navigation.provider.tsx) and #5851 (needsSettings.OpenManager.Enabled) — both are merged in here since there's no single upstream branch with both yet, so this diff includes #5851's changes inline until that one merges. Only the last commit ("Gate OpenManager's nav and page on the switch") is net-new here.Feature build: N/A on its own, same reasoning as #5851/#5852 — pairs with that PR to make the switch fully functional.